@blink1073 - just confirming - is this now ready to go in from your perspective?

Actually I just finished the setupbase.py file in the other PR, porting here and jupyter-packaging now.

Cool, thanks

Boom, this and jupyter/jupyter-packaging#19 are gtg.

Tested this out in the classic notebook, and the nbextension/static/index.js file is not getting included in the package wheel. I'm checking it out...

I'm creating a new cookiecutter output with this branch, then doing python setup.py bdist_wheel --universal --verbose, and indeed the nbextension/static/index.js is not getting included. However, that index.js file is getting generated, and if I make another wheel by running the command again, we are getting the index.js file. So it seems there may be a step out of order? Checking...

Building an sdist python setup.py sdist correctly includes the index.js, and then building a wheel after that also correctly includes the index.js.

It appears that the problem is that the get_data_files at https://github.com/blink1073/opinionated-widget-cookiecutter/blob/4e6a4275370e422e30ec9ed22d4c6cc2fd7bf76a/%7B%7Bcookiecutter.github_project_name%7D%7D/setup.py#L53 runs before npm has run its course generating the files, so only the extension.js file is picked up, not the generated index.js file.

I don't run into this with ipywidgets because I explicitly run the js build step before the packaging step (i.e., we don't have python run the js build step). It does seem that there is a difference here between building a wheel vs building an sdist.

@blink1073 - thanks! Did you not use the glob module because it only supports ** in python 3.5 or later? (https://docs.python.org/3/library/glob.html).

Also I noticed that a bunch of the placeholders (like name = '{{ cookiecutter.python_package_name }}') are gone - I can go through and put those back in a PR to your PR (maybe Monday).

Yes, ideally we'd all be using Py35+ 😄. I had attempted to get glob2 to work as a setup_requires, but I couldn't figure out how to make that work.

I recently made globmatch. Maybe it can be useful while waiting for the future?

I had attempted to get glob2 to work as a setup_requires

Oh right, that will be a general problem for any non-standard package I guess.

I'll make sure that the glob behavior gives the intended files in Jupyter Lab as well.

Okay, in c2b50d2 I updated the package data to not include the root (they are relative file paths from the package root), and ensured that staging/* did not match staging/build/index.out.js, but themes/** did match recursively in JupyterLab.

I reinstated node_modules recursion for the mtime check, since it didn't have a measurable effect on time python setup.py sdist in jlab.

Note that the glob change is consistent with the std lib:

In [7]: glob.glob('./jupyterlab/staging/*', recursive=True)
Out[7]:
['./jupyterlab/staging/node_modules',
 './jupyterlab/staging/index.js',
 './jupyterlab/staging/webpack.config.js',
 './jupyterlab/staging/yarn.lock',
 './jupyterlab/staging/package.json',
 './jupyterlab/staging/yarn.js',
 './jupyterlab/staging/build']
In [8]: glob.glob('./jupyterlab/staging/**', recursive=True)
Out[8]:
['./jupyterlab/staging/',
 './jupyterlab/staging/index.js',
 './jupyterlab/staging/webpack.config.js',
 './jupyterlab/staging/yarn.lock',
 './jupyterlab/staging/package.json',
 './jupyterlab/staging/yarn.js',
 './jupyterlab/staging/build',
 './jupyterlab/staging/build/index.out.js',
 './jupyterlab/staging/build/af7ae505a9eed503f8b8e6982036873e.woff2',
 './jupyterlab/staging/build/674f50d287a8c48dc19ba404d20fe713.eot',
 './jupyterlab/staging/build/main.bundle.js.map',
 './jupyterlab/staging/build/0.bundle.js.map',
 './jupyterlab/staging/build/b06871f281fee6b241d60582ae9369b9.ttf',
 './jupyterlab/staging/build/package.json',
 './jupyterlab/staging/build/0.bundle.js',
 './jupyterlab/staging/build/912ec66d7572ff821749319396470bde.svg',
 './jupyterlab/staging/build/fee66e712a8a08eef5805a46892932ad.woff',
 './jupyterlab/staging/build/main.bundle.js']

While I agree with the purpose of the glob change, it breaks the following patterns:

• Pattern dir doesn't match dir/subdir/file. Now pattern needs to be dir/**/*.
• Similarly, pattern dir doesn't match dir/.
• Pattern **/dir doesn't match /dir/file. Now pattern needs to be **/dir/**/*.

I'll iterate on the algorithm on Monday to see if I can include the wanted behavior.

I would say match the behavior of the stdlib so we don't get confusion when we can switch later. If you want to recurse into a directory, you must use **:

In [3]: glob.glob('./jupyterlab/staging/', recursive=True)
Out[3]: ['./jupyterlab/staging/']

(at least in this location, you are free to be more opinionated in globmatch of course 😄)

BTW: I see that setuptools excludes data files from package data. That's probably something that we should do as well.

I would say match the behavior of the stdlib so we don't get confusion when we can switch later.

That behavior is still quite strange. Try glob.glob('./jupyterlab/staging/**', recursive=True). You will get both files and directories, while you would expect it only to match subdirectories by the same logic as before. If you want to match that behavior as well you will need further changes to the code.

I'll iterate on the algorithm on Monday to see if I can include the wanted behavior.

I did push changes to the globmatch repo that achieves this, but holding back on pushing it here.

Okay, rather than try and meet the 3.5 spec entirely, I'd say let's inline globmatch as is and refer to it.

I'm flip-flopping a little here: I was considering the case where you have a [pre]<wildcard>[post] pattern that matches directories. Should that recurse or not? Example:

test*: Should it match tests/conf.py ? Or only files with prefix test like test_foo.py? The behavior should be the same as for *.

I'd say that test* should not match tests/*.

Pushed changes I think I'm okay with: Any directory match basically does nothing. Add an explicit trailing /** to include all the sub-content of a directory (in line with stdlib and git).

PS: For globmatch I put the behavior behind a flag.

Tested current version of pythreejs and it seems to work well (need to add a custom step to update packages since __init__.py files are auto-generated as well). The only outstanding issues I have are:

• Should / do we respect setuptool's exclude_package_data field?
• Should we always import and use setuptools for better consistency across commands?

I don't think we should use setuptools everywhere. For example, it doesn't handle data_files properly. I agree with exclude_package_data.

For example, it doesn't handle data_files properly.

I thought that the bdist_egg_disable fixed this issue. but yeah I'm happy to merge as is (and take that discussion separately if it is still an issue).

Regarding exclude_package_data: It will currently be applied by build_py, but it doesn't support ** in patterns. Considering punting since it seems non-trivial to get it right.

Considering that, I'll merge for now and transfer the repo.

Sweet! Pleasure doing business with you 😄.